feat(distributed): workers no longer need inbound ports (worker tunnel) - #11812
Open
localai-org-maint-bot wants to merge 79 commits into
Open
feat(distributed): workers no longer need inbound ports (worker tunnel)#11812localai-org-maint-bot wants to merge 79 commits into
localai-org-maint-bot wants to merge 79 commits into
Conversation
Starting a Postgres and a NATS container per spec cost roughly 48 minutes of startup across the 213 specs behind SetupInfra, which is why this suite was never wired into CI. Containers move to BeforeSuite and isolation comes from CREATE DATABASE, which the dbName argument already described. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
A failed CREATE DATABASE panics out of the assertion before closeDB runs, leaking a pgx pool per attempt. With --flake-attempts 5 that exhausts postgres:16-alpine's 100 connection slots, at which point the cleanup path's own Expect fails the spec and one hiccup cascades across the suite. Scope the admin handle so the panic unwinds through defer closeDB, and let cleanup use a fallible tryAdminDB that reports rather than asserts. Register DeferCleanup immediately after CREATE so a later failure cannot leave the database behind, and warn on TestInfra that the container handles are now suite-wide. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The WebSocket log handler writes its "initial" batch before it calls Subscribe, so a line appended the instant that batch arrives lands in the circular buffer with no subscriber to receive it. Three backend-logs specs append exactly there and then wait out a 5s read deadline; once a gorilla read hits its deadline the connection is unusable, so the spec cannot retry. `--focus='Worker WebSocket log streaming' --repeat=25` failed on attempt 17 with nothing else running, which is far too often to wire into CI. Add BackendLogStore.SubscriberCount, resolving a model ID by the same exact-key and replica-prefix rules Subscribe uses, and have the specs poll it until the handler has attached. Nothing in production calls it and no assertion is weakened; the handler's own snapshot/subscribe window is left as it is, being a production streaming question rather than a test one. Verified with 60 repeats of the WebSocket specs and three consecutive --randomize-all runs of the whole distributed suite, all at --flake-attempts 1: 239 of 240 specs pass in about 80 seconds. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
… works around Three corrections from review of the previous commit. The lock-order comment on SubscriberCount claimed no path takes s.mu and a buffer lock together. Subscribe does exactly that, holding s.mu.RLock across replica registrations that take buf.mu. State the rule that is actually true — s.mu precedes any buffer lock, so counting after releasing it preserves the order — and say what follows from it: the total is a sample, not a snapshot. waitForLogSubscriber read as general-purpose but unblocks on the first registered subscription. Subscribe attaches the exact-key buffer and each replica buffer one at a time, so for a replicated model the count goes positive while later replicas are still unattached and the race survives. Rename it waitForSingleLogSubscriber, document that it holds only where Subscribe resolves to one buffer, and assert on exactly 1: misuse then fails loudly on the count rather than going quietly back to being flaky. Taking the expected count as a parameter was the alternative, but that makes callers predict a store-internal number and an under-count fails the same silent way as the original bug. The snapshot-then-subscribe race had no artifact outside a report, and review found a second site carrying it. Mark both handlers identically, including the point that swapping the two calls duplicates rather than drops and so is not the fix. The race itself is left alone; this branch stays test infrastructure. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The suite has never run in CI, so 239 specs across 32 files were verified only by hand. Path-filtered to distributed code, advisory until it earns a track record, and with flake retries at 1 rather than 5 so nondeterminism surfaces instead of being retried away. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The path allowlist covered 13 of the 99 packages the suite reaches. Commit 1dc3aee touched core/config, core/services/modeladmin and core/backend and matched no entry, so it would have merged without running the very specs that cover it. Use the paths-ignore denylist tests-e2e.yml already uses. Disable the testcontainers reaper: the runner is ephemeral, so the reaper buys nothing and its unpinned image was pulled mid-suite, defeating the pre-pull. Drop continue-on-error, which no other workflow uses and which reports a failed run as green. The job is advisory by staying out of branch protection instead. Pin Go to 1.26.0 to match go.mod, and add the tmate-on-failure step. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Runs local-ai as real child processes, one per frontend replica and one per worker, against containerised infrastructure. The in-process suites cannot express frontend-replica failure: there is no process to kill and no real HTTP boundary between a worker and the frontend it registered with. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Restarting a frontend replica must not move it: workers read LOCALAI_REGISTER_TO once at boot and never re-resolve it, so a replica that returns on a fresh port is unreachable by the workers that registered with it. startFrontend now takes the port, with <= 0 meaning "allocate". Process logs are opened for append rather than truncated, so a restarted process cannot erase the log of the instance that died, which is the log a failover post-mortem needs. The post-SIGKILL wait is bounded, so one stuck child no longer becomes a suite-wide timeout that names nothing. Stop is nil-safe because Start returns a nil cluster after stopping itself. Start's doc comment no longer claims to wait for worker registration; that needs an authenticated admin session, so it now says callers must poll /api/nodes themselves. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The register handler answers 201 both for "user created, here is your session" and for "this email already exists", so the status code cannot tell a fresh registration from a repeat one. Key on the session cookie instead and fall through to login when it is absent. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
… secret
Session rows are keyed by an HMAC of the token under a secret generated
per instance into {DataPath}/.hmac_secret. The replicas shared that
secret only because they shared a working directory, and that directory
was the source tree. Give each frontend LOCALAI_DATA_PATH under its own
baseDir and pin LOCALAI_AUTH_HMAC_SECRET, so a session minted at one
replica resolves at every other one by construction.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…ness
The point of running LocalAI as real child processes is to be able to take
one away. Add KillFrontend (SIGKILL, the lost replica), StopFrontendGracefully
(SIGTERM, the rolling update), KillWorker, RestartFrontend and FrontendAlive.
RestartFrontend pins the dead replica's original port. Workers read
LOCALAI_REGISTER_TO once at boot and never re-resolve it, so a replica that
returns on a fresh port is unreachable by exactly the workers that registered
with it and the failover under test never happens.
It also wipes the replica's data directory, so the process comes back with
empty local state and has to rehydrate node, session and job state from the
shared Postgres and NATS. Reusing the directory would model a pod with a
persistent volume and hide the class of bug these tests exist to find. That
is only safe because the harness pins LOCALAI_AUTH_HMAC_SECRET; otherwise the
wipe would take {DataPath}/.hmac_secret with it and every session minted
before the restart would 401 afterwards.
FrontendAlive consults the reaper's exited channel before signal 0: a child
that has died but has not yet been waited on is a zombie, and signal 0 to a
zombie succeeds, which would report a dead replica as alive.
The new specs cover argument validation only. Killing, stopping and
restarting a live process needs a built binary plus Postgres and NATS, so
those paths stay unexecuted until the failover suites land.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…he wipe Review round 1. Comments only, plus one guard. The note on Process.alive claimed the exited check closed the zombie window. It does not. The reaper closes exited only after Cmd.Wait returns, and Wait marks the os.Process done before returning, so exited being closed implies signal 0 already errors and the branch cannot fire earlier than the one it precedes. The window between the child exiting and waitid collecting it stays open in both versions, and the only real mitigation is for callers to poll with Eventually rather than sample once. Keep the check as hygiene, say what it actually does, and say it again on the exited field, so nobody reads the old claim and drops the Eventually. Record what the cold wipe destroys. The harness sets no LOCALAI_STORAGE_URL, so the object store is a directory under DataPath, and quantization and fine-tune outputs live there too. Postgres keeps the job row; the artifact it points at does not survive the restart. A spec that asserts otherwise will fail for a storage reason wearing a failover costume. Tell callers to let a graceful stop finish before restarting: RestartFrontend terminates with SIGKILL, so pairing it straight after StopFrontendGracefully cuts the drain short and silently converts the rolling-update case into the crash case. Refuse to wipe when the cluster has no work dir. frontendDataDir is relative when baseDir is empty, so a Cluster built by some future test helper without one would have RemoveAll walking frontend-N/data inside the source tree. The guard sits before terminate, so a refusal leaves the cluster as it was. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Tasks 4 to 6 built a harness that runs local-ai as real child processes, but none of it had ever started a process: every spec so far returned inside argument validation. These two specs are the first to run it against a real binary, a real Postgres and a real NATS. Two frontends against one database both see a worker that registered through only one of them. Every failover spec assumes this, so it is asserted first. One admin session is minted at frontend 0 and reused for both replicas rather than registering per frontend. The auth routes share a five-per-minute-per-IP limiter and all e2e traffic is 127.0.0.1, so a session per frontend would exhaust the budget as soon as a spec needs a third one. Reuse is sound because sessions live in the shared Postgres and the harness pins one HMAC secret across replicas; frontend 1 answering /api/nodes with 200 on a cookie minted at frontend 0 is what proves it. The binaries are resolved before SetupInfra so a missing build skips without first provisioning a database the skip would then have to tear down. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The Cluster label partition is these two specs and nothing else, so a missing binary skipped the entire job. Ginkgo exits 0 on skips, so a build step that broke or moved its output would have left the job reporting "0 Passed | 2 Skipped" and going green without ever starting a cluster: the silent pass this suite exists to make impossible. Skipping stays the local default, which is the right courtesy for someone who has not run `make build`, but LOCALAI_E2E_REQUIRE_BINARIES turns it into a failure that names the missing path and the target that builds it. A value that is set but unparseable counts as on, since reading it as off would restore the very skip it disables. Failures also name themselves now. The roster poll kept returning a bare nil on error, so a 401 at the second replica, a decode failure and "the worker never registered" all presented identically as an empty list. It now retains the last error and the last roster and reports whichever happened, through a lazily evaluated Gomega description that costs nothing until something fails. Finally, the two-frontend spec no longer depends on the harness to mean what it says. It asserts an unauthenticated GET /api/nodes at frontend 1 is refused, which observes the admin gate instead of assuming it, and it compares the worker's registration id across the two replicas rather than its name. A future harness that registered every worker with every frontend would have kept a name-only assertion green while it quietly stopped proving anything about shared state. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The previous round made a missing binary fail instead of skip, but only when a workflow remembered to set LOCALAI_E2E_REQUIRE_BINARIES. That leaves the silent pass one forgotten line away: the Cluster label partition is two specs, Ginkgo exits 0 on skips, and a job that skips both reports "0 Passed | 2 Skipped" and goes green having never started a cluster. So the polarity is inverted. Binaries are required whenever CI is set, which GitHub Actions always does, and the flag now exists to force the requirement OFF rather than to be remembered ON. A local developer sees no change, since CI is unset in an ordinary shell and a missing binary still skips with a message naming the path and how to build it. off, no, n and disabled are honoured as off; ParseBool rejects them, and reading a word that unambiguous as its opposite would be a worse trap than the one this removes. Also correct a claim the previous commit message got wrong. Comparing the worker's registration id across the two replicas does not pin the topology: NodeRegistry.Register looks a node up by name and preserves the existing id, and both replicas read one Postgres, so registering the worker with every frontend would yield identical ids too. The assertion is still worth keeping for what it does catch, a replica answering from its own registry or database instead of the shared one, and the comment now says that and nothing more. The topology fact moves to where someone would break it: a note on LOCALAI_REGISTER_TO recording that workers register with frontend 0 only, that the cross-replica specs depend on it, and that nothing in those specs can detect a change to it. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…plicas Four scenarios with no prior equivalent: killing a replica must not disturb a worker that never depended on it, a cold-restarted replica must rehydrate the roster from shared state and keep accepting the worker's heartbeats, a dead worker must settle to offline on every replica, and two replicas registering a worker each must converge on one roster. The timings are measured, not assumed. Node liveness is heartbeat freshness, so the only eviction path is StaleNodeThreshold (60s) plus one HealthCheckInterval tick (15s), and neither is reachable from the CLI. A worker whose registrar was killed was observed going offline at 74.2s. Every window here is sized to outlast that, because an assertion that expires before the system could have reacted proves nothing. Two assertions are deliberately unlike the obvious form. Statuses are compared for equality against a probe that returns a sentinel on error, rather than asserting a name is absent from the healthy list: the list probe returns nil on any error, and "does not contain" is satisfied by nil, so a 401 at the second replica would have passed while observing nothing. And a killed worker is required to settle to exactly offline, because it first flaps to unhealthy at ~8s and back to healthy at ~14s, which any not-healthy matcher would accept. SpreadWorkerRegistrations is new, off by default, and exists so the racing spec is a race: the harness otherwise points every worker at frontend 0, which would have left that scenario asserting on two sequential writes through one process. The default is unchanged because the baseline specs depend on it. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…r windows The two specs that assert a healthy worker stays healthy were pure negatives: they say nothing happened. A cluster whose health checking had wedged, by leaking the advisory lock the monitor takes at health.go:110, would freeze the roster and satisfy both while observing a corpse. Kill the worker once the window closes and require the roster to settle it to offline, so the preceding Consistently is a statement about behaviour rather than about a stopped clock. Applied to the cold-restart spec as well as the peer-death one: a restart is exactly the event that could leave a replacement unable to check anything. Document the hazard that can make an offline assertion hang. The staleness branch skips a node already marked unhealthy (health.go:153-155), a skip meant for nodes an operator took down, which also swallows the flap: an unhealthy mark landing after the heartbeat goes stale means MarkOffline is never called and the node stays unhealthy forever. Name the file and line at the assertion, and have the failure message say so when the roster shows a node stuck there, so a timeout sends the reader to LocalAI rather than to the harness. Stop calling the two-replica registration spec a race. Start spawns workers sequentially and the registrations land about a second apart; it is a shared-roster identity test, and saying otherwise invites someone to trust it for something it does not check. WorkerRegistrar now bound-checks its index like every other index-taking method here. It answered 0 for an out-of-range worker, and 0 is a real frontend index, so the failure mode was a spec killing the wrong replica. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Add test-e2e-cluster and a second CI job that runs it. The cluster specs spawn local-ai as real child processes and kill them, so they need a built binary; keeping them in their own job means the fast in-process suite is not held behind that build. The binary is built with a stubbed core/http/react-ui/dist. A single index.html satisfies the go:embed in core/http/app.go, and this suite drives the HTTP API only, so the job skips a Node and Vite install entirely. The job runs serial and pins --flake-attempts 1. Each Ginkgo process would otherwise get its own PostgreSQL and NATS container while every spec spawns two or three children, and a retry would hide exactly the nondeterminism the suite exists to catch. Measured at 8m39s over three runs, hence a 25 minute job timeout and a 20 minute Ginkgo timeout. LOCALAI_E2E_LOG_DIR points inside the workspace so the per-process logs upload as an artifact on failure; they are the only way to read a cluster failure. LOCALAI_E2E_REQUIRE_BINARIES is set explicitly even though CI already implies it, because a skipped cluster spec is indistinguishable from a passing one and this job's whole value is that it cannot go green without starting a cluster. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Ginkgo exits 0 when a label filter matches nothing, so a refactor that
renamed or dropped Label("Cluster") would have left the job reporting
"Test Suite Passed" having started no cluster. LOCALAI_E2E_REQUIRE_BINARIES
does not cover that case: it only fires inside a spec that is already
running. Add --fail-on-empty to both distributed targets.
Drop -r from test-e2e-cluster while here. All six Cluster specs live in the
top-level package, and the cluster subpackage contributes nothing under this
filter by design, so recursing only widened the blast radius. test-e2e-
distributed keeps -r: it must reach the eight argument-validation specs in
that subpackage.
Raise the cluster job to 45 minutes, matching its sibling. The 20 minute
Ginkgo timeout bounds the suite alone; the job timeout must also cover setup,
which is the larger and more variable half here: cold-cache module download,
protoc and protogen-go, a full build of ./cmd/local-ai and a separate test
compile, realistically 8-12 minutes on a 4-vCPU runner. At 25 minutes the
runner would have hard-killed the job before Ginkgo could report which spec
hung, which is the red-with-no-evidence outcome that gets suites disabled.
Also move upload-artifact to @v7 with the rest of the repo, and note on the
react-ui stub step that it must go if a spec ever asserts on a UI asset,
since a developer box has a real dist/ and would not catch that locally.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Two Make targets, a flake-budget variable and two environment variables landed with no way to discover them. CONTRIBUTING.md now tells a contributor how to run both suites, what each costs and which variables steer the cluster one. .agents/building-and-testing.md records the decisions that are easy to undo by accident: suite-scoped containers, the shared NATS bus and what that means for a new spec, BeforeSuite over SynchronizedBeforeSuite, the label split, --fail-on-empty, the binary gate, the flake budget of 1, the coverage exclusion, and why the cluster suite's long waits must not be shortened. .agents/ci-caching.md lists tests-e2e-distributed.yml in its paths-ignore inventory; the workflow already pointed readers there, so the cross-reference was dangling. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
--flake-attempts is total attempts, not retries: ginkgo v2.29.0 sets maxAttempts = FlakeAttempts and loops attempt < maxAttempts, and the flag's usage string reads "0 - failed tests are not retried". At 1 there is no retry at all, so "retries a failing spec once" was false in CONTRIBUTING.md and implied in .agents/building-and-testing.md. Both now say each spec runs once, and cite the source so the next reader need not re-derive it. Also restores the React-UI stub rationale, which is load-bearing because a spec asserting on a UI asset passes locally against a real dist/ and is served the stub in CI; explains why 213 and ~240 differ; records that the workflow also triggers on master pushes, where paths-ignore does not apply; and completes the LOCALAI_E2E_REQUIRE_BINARIES value table, including that any unparseable value reads as ON. In .agents/ci-caching.md the stale "13 of those 20" figure now carries its qualifier inline rather than in the following sentence. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Review of the whole branch found five comments that would send a reader to the wrong place, plus three smaller inaccuracies. Nothing here changes behaviour. The KNOWN RACE note on both backend-log WebSocket handlers said the fix needs an atomic snapshot-plus-subscribe "under the store lock". It does not: BackendLogStore.mu guards only the buffers map, and AppendLine enqueues and fans out under the per-buffer buf.mu. Whoever took the store lock would ship and the race would survive, so both notes now name buf.mu and say what s.mu does and does not exclude. Two comments in the cluster harness quoted Eventually(c.FrontendAlive) .Should(BeFalse()). FrontendAlive takes an index, so Gomega rejects that with "requested 1 arguments but received 0". Both now quote the closure form the specs actually use, and say why the closure is needed. proveHealthCheckingIsAlive claimed to prove the health monitor ran for the whole preceding window. It proves the monitor was alive at the end of it, and inferring backwards needs any wedge to be sticky. In the peer-replica-death spec that inverts: health checks are single-flighted by a session-scoped pg_try_advisory_lock, the spec SIGKILLs the replica that may hold it, and until Postgres reaps the session the survivor acquires nothing and checks nothing silently. Consistently(healthy) can then pass because nothing was checking, with the positive control still succeeding once the lock frees. The doc now states what is proven, names that gap, and says the assertion is a floor rather than a proof. The Makefile still called DISTRIBUTED_TEST_FLAKES a retry count, which is what seeded that error into the two docs just corrected against it, and the workflow called the 15s window a reconcile tick when the mechanism is HealthCheckInterval in the node health monitor. Also: the cluster suite measured 509.1s / 509.8s / 512.3s, so about 8m30s and not the 8m39s/8m40s three files claimed; the dead-worker spec title implied two independent detectors when both probes read one advisory-lock-serialised verdict out of the same row; and the sanitizeDBName length assertion used <= 50, which an empty string also satisfies, where the invariant for an over-long input is exactly 50. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The closure note in cluster/failure.go quoted a Gomega error that Gomega does not emit. Describe the argument-count failure and the Eventually().WithArguments() hint instead, so nobody greps for a string that never appears. The advisory-lock note in cluster_failover_test.go called the wedge window unbounded. A SIGKILLed local child closes its socket at once, the Postgres backend reads EOF and is reaped in milliseconds, so the mechanism bounds the window tightly. Say bounded, and keep the low probability but real framing, which was right. The workflow comment attributed HealthCheckInterval to core/services/nodes/health.go. It is declared in core/config/distributed_config.go:64; health.go only carries the ticker on the unexported checkInterval. Point a debugger at the right file. Comments only, no behaviour change. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
| // invariants still apply: non-empty, at most 72 bytes, no NUL. The | ||
| // acknowledgement is deliberate rather than incidental, so a future | ||
| // tightening of the policy cannot break every failover spec at setup time. | ||
| adminPassword = "e2e-admin-password" |
| defaultAdminEmail = "admin@e2e.local" | ||
| // testHMACSecret is shared by every frontend so a session minted at one | ||
| // replica validates at all of them. See the note in startFrontend. | ||
| testHMACSecret = "e2e-cluster-hmac-secret" |
| } | ||
| name := frontendName(i) | ||
| dir := c.frontendDir(i) | ||
| if err := os.MkdirAll(filepath.Join(dir, "models"), 0o755); err != nil { |
| if err := os.MkdirAll(filepath.Join(dir, "models"), 0o755); err != nil { | ||
| return nil, fmt.Errorf("creating %s dirs: %w", name, err) | ||
| } | ||
| if err := os.MkdirAll(filepath.Join(dir, "backends"), 0o755); err != nil { |
Comment on lines
+188
to
+192
| cmd := exec.Command(c.opts.Binary, "run", | ||
| "--address", fmt.Sprintf("127.0.0.1:%d", port), | ||
| "--models-path", filepath.Join(dir, "models"), | ||
| "--backends-path", filepath.Join(dir, "backends"), | ||
| ) |
| logPath := filepath.Join(c.opts.LogDir, name+".log") | ||
| // Append rather than truncate: a restarted process reopens the same path, and | ||
| // the log of the instance that died is the one a failover post-mortem needs. | ||
| f, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o644) |
| logPath := filepath.Join(c.opts.LogDir, name+".log") | ||
| // Append rather than truncate: a restarted process reopens the same path, and | ||
| // the log of the instance that died is the one a failover post-mortem needs. | ||
| f, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o644) |
| } | ||
|
|
||
| func copyExecutable(src, dst string) error { | ||
| data, err := os.ReadFile(src) |
| if err != nil { | ||
| return fmt.Errorf("reading %s: %w", src, err) | ||
| } | ||
| if err := os.WriteFile(dst, data, 0o755); err != nil { |
| if err != nil { | ||
| return fmt.Errorf("reading %s: %w", src, err) | ||
| } | ||
| if err := os.WriteFile(dst, data, 0o755); err != nil { |
Replicas need to find each other to relay worker traffic, and nothing in the tree recorded a replica's address. The advertised address is discovered by opening a UDP socket toward PostgreSQL and reading back the local address, which yields the interface every replica demonstrably shares without asking an operator to configure one. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…ble addresses DiscoverAdvertisedAddr promised to return an error rather than a fallback no peer can dial, but only rejected an unspecified address. With PostgreSQL on the same host or pod as a replica, which is compose, single-node and any sidecar layout, the route to it is loopback, so every replica advertised 127.0.0.1 and a peer dialling that reached itself. Loopback, link-local and zoned source addresses are now rejected with an error naming the remedy, and a port outside 1-65535 is rejected before it becomes an undialable address. Liveness was also measured on each replica's own clock: Register and Heartbeat stamped last_seen from the Go process, and Live compared those rows against the reading replica's time.Now(). Skew therefore shrank or stretched the window by writerBehind+readerAhead, evicting healthy peers or keeping dead ones. Both sides now use the database clock, which is the one clock every replica demonstrably shares. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Returns on the first direction to finish and closes both sides so the other unblocks; a sequential copy deadlocks on any protocol where the far side speaks first. EOF and use-of-closed are normal termination, not errors. The fourth spec covers a peer that stops reading mid-body, the case where a copy is parked in Write rather than in Read. The other three tear down an idle splice and pass even against a Splice that closes only one side. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
go-yamux/v5 matches none of its errors against net.ErrClosed, so the classifier reported an ordinary teardown as a failure: when the session has gone away, the FIN that Splice's own Close writes returns ErrSessionShutdown, and a stream torn down under a live copy surfaces as ErrStreamClosed or a reset. Splice owns that Close, so it owns the errors it produces; the sentinels are named here rather than injected by the caller, which would make a forgotten classifier reintroduce the same bug silently. Cover the error half of the contract, which no in-memory pipe could reach: a scripted stream now feeds Splice a genuine transport failure and each closed-stream ending in turn. Replacing the tail of Splice with "return nil" passed every previous spec. Also assert that Splice does not return until the second direction has finished, rename a spec that promised a leak check it never made, and correct two comments that claimed more than the code did. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Matching yamux errors with errors.Is was too broad. Session.close hands every live stream ErrStreamReset wrapped around whatever killed the connection, so a keepalive timeout, a broken TCP connection or a peer that simply vanished all matched, and a relayed request that died reported a clean ending. Nothing upstream would have retried or logged it. Match the plain sentinels by identity, since only identity separates a stream that was reset from the wrapped form that means the session died. Treat a StreamError as a per-stream reset, and a GoAwayError as normal only when it carries the no-error code, read off ErrRemoteGoAway because the constant is unexported. ErrSessionShutdown needs no entry of its own; it is a GoAwayError with that code. Order matters as much as the matching: session death wraps its cause, which is routinely io.EOF or a closed socket, so the mux checks run before the generic endings. Reversing them alone puts a vanished peer back to nil. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Upgrades to a WebSocket, wraps it as a yamux server session and hands it to the caller. Rejects before upgrading so an unauthenticated dial sees a 401 rather than a WebSocket error, which is what the route-coverage test asserts. The adapter keeps the reader of a partially consumed message across Read calls. yamux reads through a 4 KiB bufio.Reader, so a small-payload test cannot see a dropped message tail; the framing specs drive the adapter directly with buffers smaller than the message. An empty configured token authorizes nobody here, unlike the worker file transfer server's check: this route is registered in every deployment, so failing open would publish an unauthenticated mux. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…gets
Review round 1 on task 5. Eight non-blocking items, all addressed.
The classifier read Remote in two predicates with a report-by-default
fallthrough behind them, so reverting either read left the whole suite green:
the error reached the same answer down the other path. A correctness argument
that rests on mutation evidence cannot afford a shape that cannot be mutated in
pieces, so the two predicates collapse into one muxVerdict deciding each error
type once. Falsifying either Remote read now reddens exactly one spec.
Three claims the comments made loudly and nothing tested:
- clearing the header read deadline before the splice. Deleting the clear
left all 49 focused specs green, while in production it is the difference
between a relayed response that streams for an hour and one that dies after
fifteen seconds of quiet;
- the open budget bounding the open and nothing after it;
- closing the worker-side stream when the acceptance reply cannot be
written, which leaks one stream on the worker per failure.
All three are pinned now. The first two share a spec that sets both budgets to
50ms and then watches the conversation outlive them by ten times, which is an
assertion about an event that must not happen and so is the one wait a channel
cannot replace. The third drives the relay with a peer stream that delivers a
request and then fails every write, because no pair of live yamux sessions can
be made to fail that write on cue.
The disjoint-vocabulary argument was specced for the accepted frame only. Both
refusal directions are covered now, and asserted as "not one of the other hop's
sentinels" rather than merely "an error", since reading a relay refusal with the
tunnel's reader always errors and the question is whether it errors as the wrong
thing.
The open budget stays non-configurable, and says so: the number that matters is
how long the original client will wait, which is not known on this side and is
not something a deployment-wide constant can stand in for. The honest fix is the
caller's remaining budget travelling in the request frame, which belongs to the
dialler that has the budget.
Two comments corrected: nothing deadlines the peer stream after the clear, so
the tunnelled protocol's own deadlines cannot be what justifies clearing it; and
the membership sweep deletes departed replicas but reports only how many, so
identifying them is work that would have to be done, not knowledge waiting to be
plumbed. Recorded at muxVerdict: a remote RST that does not ride a
typeWindowUpdate frame yields the bare sentinel and is still silenced, which is
unreachable between two go-yamux peers but keeps the new rule from reading as
unconditional.
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude Opus 5 [claude-code]
The tunnel, the fence, the registry and the relay were all built and none of them carried a byte: every dial from the frontend still went to the address a worker registered. This is where that stops. One WorkerDialer resolves where a worker's tunnel is held, opens a stream on it locally or relays through the owning replica, and hands back a conn past both handshakes; gRPC, the file stager's HTTP client and the log-streaming WebSocket are all pointed at it. A worker's address stops being somewhere to connect to and becomes the name of which backend process a stream is for. It still appears in URLs, logs and errors, because that is what identifies the process; what it no longer decides is where the bytes go. Nothing falls back to dialling it. BackendClientFactory now has exactly one method, NewClientForNode, and returns an error where there is no way to reach the worker. The direct-dial constructor was removed rather than kept beside it, because leaving one on the interface keeps the bypass one word away from every call site that holds an address, which is all of them. The second construction path is closed too. DistributedModelStore built remote models with a nil client, and pkg/model.Model.GRPC then dialled the raw address lazily on first use - reached in production by ShutdownModel's Free and by the backend monitor's Status. Those models now carry the tunnel-backed client, and a model that cannot be given one is logged and not listed. Four conditions stay unmixable, and one path produces absence: the dialer answers ErrNoConnection only where Owner's liveness join did. A peer that will not answer, a stale ownership row, a worker's own refusal and a missing relay path are each reported as themselves. This matters because nodes ACTS on absence, and the collapse would have it reclaim the models of a worker that is connected and busy. That is not hypothetical. Writing the mutation for it exposed the bug in this change's own first draft: probeHealth returned bare false when it could not build a client, and tryWarmPath deletes the replica row on a false probe. A frontend whose dialer broke would have emptied node_models for the whole deployment while every model kept running. probeHealth now returns alive and probed separately, the reconciler gets a ProbeUnknown outcome that neither advances nor clears a failure streak, and the health monitor skips rather than counting a miss. Task 5 left the relay's open timeout at a fixed 15s and said so: no operator has the information to set it, because the number that matters is the original client's remaining budget, which is invisible on the relay side. The dialer has that budget, so it now states it in the relay request frame and the owner takes the smaller of the two. It can only shorten - a patient client must not be able to park a relay goroutine and a stream slot on a worker that stopped accepting. Zero is written as no budget at all, since on the far side the number zero is a caller with nothing left and would refuse healthy traffic. Seven mutations, each reddening a named spec: peer-unreachable as absence; the local-failure guard dropped; max instead of min on the budget; the nil-client model restored; ProbeUnknown falling through to the reaper; OwnerRow instead of Owner; probed collapsed into alive. The first budget spec passed for the wrong reason - a handshake deadline, not the relay - and was replaced by three that each assert one link, including one where the spec plays the owning replica and reads the budget out of the frame instead of inferring it from a clock. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…of the package Review round 1 on task 6. Five blocking findings, all with the same root: the conditions the dialer kept apart were erased one layer out, because every one of them arrived at core/services/nodes as a gRPC codes.Unavailable, which is also what a backend process that died produces. Four call sites acted on that by deleting a replica row, one of them after a single failed probe. The fifth condition is ErrNoRoute: this replica could not get a request to a worker's backend, and no claim at all about the worker. A worker's presence is its HEARTBEAT, which nodes owns; a route is a separate fact that cluster owns, and the two now differ. They differ in normal operation, not exotically: a worker that has not dialled its tunnel yet after a frontend-first upgrade is unroutable on every request while it heartbeats and serves. Two properties, both mutation-tested. Every failure to resolve or open a route carries ErrNoRoute, so a consumer has one check to make. No failure carries an absence sentinel: routeFailure is the single place that rule lives, and it keeps ErrNoConnection and ErrInstanceNotFound in the message and out of the unwrap chain, the guarantee unreachableError already made for peers. Everything else stays matchable, so ErrNotOwner and ErrPeerUnreachable are unchanged for anyone who can act on them. A worker's own refusal carries no umbrella, because a worker that answers has demonstrated it is there and that is the only real evidence on the path. Crossing the boundary needed a value, not a code. NewClientWithDialer wraps the dialer and records each outcome; LastDialError hands it back behind a narrow interface, and nodes.unroutable turns it into ErrWorkerUnroutable with the cluster sentinels still in the chain. A spec asserts a dial failing with ErrNoRoute plus ErrPeerUnreachable arrives matching all three and matching neither absence sentinel. The sweep found a fourth site the review had not named: pkg/model checkIsLoaded evicts a remote model on a connection error, and a tunnel dial failure is one. Four other reap sites were cleared with reasons - inflight and the worker authoritative pass reap only on semantic answers, scale-down is driven by last_used, abandoned loads decide on the node's heartbeat. Every fixed site also grew the opposite spec, so the new check cannot pass by never reaping. probeCache carries the reason through singleflight rather than a closed-over variable. A variable is only written by the goroutine that runs the probe, so the leader would correctly decline to reap while every joiner reaped on the leader's own observation; a mutation reproduces exactly that. The docs sentence promising LOCALAI_WORKER_TUNNEL=false restores direct dialling is gone. There is no such path, so it said the operator could take a worker dark and call it a rollback. Replaced with the upgrade order that is actually safe. The deadline spec the reviewer found vacuous now waits on the dial context's own Done channel before touching the stream, so the armed deadline has really expired; the mutation that survived for the reviewer reddens it. Nine mutations, each reddening a named spec, including both halves of isAbsenceClaim independently. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…st gRPC Re-review round 2. One blocking defect, and it was the concern I filed myself last round and mis-scoped as a future trap. It was live, and it sat on the most destructive reaping path of the five. RouteResult.Client is an InFlightTrackingClient, over a FileStagingClient when a stager is configured. model_router puts that on the cached remote model and pkg/model's checkIsLoaded asks IT whether the transport failed. Both wrappers embed grpc.Backend, which does not declare LastDialError, so the type assertion read nil and the guard added last round fell straight through to the old eviction. That eviction sends backend.stop over NATS to every node holding the model and deletes every replica row, where the other sites delete one. The spec covering it built a bare client by hand, which is why it passed while production did not. This is the third time in this task a correct fix was disarmed one layer out, so the fix is a mechanism rather than two methods. BackendUnwrapper is one line per decorator, LastDialErrorOf walks the chain, and both consumers now call it instead of each keeping its own assertion. One implementation, no per-caller policy to get wrong. Sweeping every type that embeds or holds a grpc.Backend found a third decorator the review had not named, and it is itself a reaping consumer of the same collapsed signal. ConnectionEvictingClient is built for remote models in initializers.go and its evict callback runs ShutdownModel; it fires during INFERENCE rather than on a health check, so a tunnel blip mid-request was enough to stop a model that was loaded and serving. It consults the transport first now. A locally spawned backend has no custom transport, so that path is unchanged byte for byte. Everything else touching a Backend is a consumer rather than a decorator; there is no fourth. The probe cache joiner shape is pinned. It was the right design last round with nothing holding it: the mutation back to a closed-over variable passed all 602 specs in the package. Eight goroutines coalesced on a probe that blocks on a channel now assert every joiner gets the leader's REASON and not just its answer, which is the difference between a leader declining to reap and its seven joiners reaping on the leader's own observation. The LastDialError scope note claimed an exactness it does not have at checkIsLoaded, which reads a shared long-lived client after releasing opMutex. It now says which caller is not exact, why the imprecision is accepted there, and what making it exact would cost. The four-outcome table in the docs still said a worker with no live owner is treated as absent and rescheduled, contradicting the code and the paragraph nine lines below it. None of those outcomes is absence any more, and the table says so, names the fifth, and points at the heartbeat as the thing that does decide presence. Five mutations, each reddening named specs, including the two the reviewer found surviving. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
… shape in lint Re-review round 2. One blocking item, and it was a spec I wrote: eight goroutines raced at the probe cache and nothing made them coalesce, so a straggler that missed the flight re-entered the probe and double-closed a channel. It panicked about one run in three and took the four-suite race block down. The green verification I reported was not reproducible, which means one green run was never evidence for a spec that coordinates goroutines. Its comment claimed the probe blocked until every goroutine was inside flight.Do, and that gap was exactly the panic: the comment described the design intended rather than the one written. It is deterministic now rather than tolerant. singleflight.DoChan registers its channel on an in-flight call under the group's own mutex and returns without running its function, so calling it while the leader is provably parked inside the probe joins that exact flight with no window and no dependence on the scheduler. The spec asserts the join really happened, that the joiner got the reason and not only the answer, and that the probe ran once; the entered channel is sent on rather than closed so a second probe fails an assertion instead of panicking. Twenty runs green under race against the committed code, five out of five red on the mutation back to a closed-over variable. The future-decorator gap is closed in the lint gate, but not the way the review suggested, and the reason is worth recording. HasMethod rejects inline signatures outright, its method-reference form needs a package ruleguard's own typechecker can import and that typechecker cannot import this module, and Implements tests the value method set while every Unwrap is on a pointer receiver, so it fired on all three wrappers that already had one. So the safe shape is structural instead. grpc.WrappedBackend gives the same pass-through method set plus Unwrap on a value receiver, and a decorator that embeds it is transparent by construction; forgetting stops being expressible rather than merely discouraged, which is the move loopbackService already makes in the worker. FileStagingClient and ConnectionEvictingClient embed it and their hand-written Unwrap methods are gone. The ruleguard rule then only has to catch the raw embedding, needs no type filter, and cannot misfire. It was verified to fire on a throwaway wrapper and stay silent on a correct one, and reports nothing across core and pkg with the baseline disabled. InFlightTrackingClient is the one exception and says why in a nolint: it embeds ControlBackend deliberately so that leaving an inference method unwrapped breaks the build, and WrappedBackend embeds the full interface, so adopting it would silently restore pass-through for every inference method and delete that guarantee. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…y-proof I reported that enabling gocritic pushed make lint past 600s. That was wrong, and it was wrong in a way worth naming: those runs happened right after I changed pkg/grpc's and core/services/nodes' interfaces, so the Go build cache was cold for essentially the whole repository including every backend, and test suites were running concurrently on the same machine. I attributed a cold-cache full-repo typecheck under load to the linter I had just enabled, and raised it as a cost without ever timing it against a baseline. A number with no control is not a measurement. Measured properly, with the golangci cache cleaned before every run and isolated GOCACHE directories for the cold ones so the shared cache was not wiped: warm, base 15s then 7s and current 8s then 7s; cold, base 87s and current 78s running base first, base 136s and current 79s running current first. The spread between the two cold base runs is larger than any gap between base and current, so gocritic with only the ruleguard checker costs nothing measurable. So the rule stays, unscoped. Scoping it to core and pkg was the fallback for a cost that does not exist, and adding that configuration would buy nothing. The one override gets the protection it needs instead. InFlightTrackingClient's nolint is exactly the kind of thing a later reader tidies away, so it now opens by saying not to, and states what breaks rather than what is intended: WrappedBackend embeds the full Backend interface, so adopting it there would promote every inference method as untracked pass-through, the build would stay GREEN, and in-flight accounting would silently stop covering whatever was added next. WrappedBackend's own doc carries the counterpart warning so a reader arriving from either side finds it. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
A worker now opens no listener on a routable interface and states no endpoint at registration. Backend processes and the file-transfer server bind loopback, and the frontend reaches both through the tunnel the worker dials. The bind address is built from loopbackHost, the same constant the tunnel's grpc tag dials, so "the worker binds where its tunnel dials" is one fact in one place rather than two literals that can drift. All three advertisement sites are closed, not one: the registration body, RegisterNodeRequest, and the per-backend address in the install reply. That third one was hiding a live bug. stopModelExact refuses a stop whose ExpectedAddress does not match what the worker recorded for the process. The worker recorded 127.0.0.1:port; handleBackendInstall reported advertiseHost:port; the router stored the reported one and sent it straight back. On any worker whose advertise host was not 127.0.0.1, every acknowledged model stop failed with an address mismatch. Nothing caught it because the e2e harness set LOCALAI_ADVERTISE_ADDR=127.0.0.1, which made the rewrite a no-op. Removing the rewrite makes the two strings the same by construction. The brief was wrong about two of the four functions it called dead. effectiveBasePort is the base of the backend port allocator and resolveHTTPAddr is the file server's bind address; deleting them would have deleted the port allocator and the file server. Only the two advertise* helpers were dead, and addr_test.go is rewritten rather than deleted, because the port arithmetic it pinned still needs pinning. NodeModel.Address survives with a narrowed meaning and is renamed WorkerLocalAddress, along with the install reply field that feeds it. The frontend still has to say WHICH backend process on a worker it means, and the port in this string is how it says it: it travels as a stream target and the worker dials its own loopback. The gorm column and the json key stay "address", so neither a migration nor an API break rides along. Every fall-back to the node's address is gone. installBackendOnNode now errors when a worker reports success without naming one, because substituting the now-always-empty node address would name an empty target, and the worker refuses that as an invalid stream, which is classified as the worker answering about its backend. That is the "a present worker reads as something it is not" class this phase forbids. DistributedModelStore.Range had the same shape and was already wrong: it built each remote model's client from the node's base gRPC port, never the port a backend process listens on, so Free and Status went to the wrong place. It uses the replica's address now. BackendNode.Address and HTTPAddress are kept but made provably inert: no writer, no reader that acts on them, and Register force-clears both on re-registration so an upgraded worker's stale advertisement does not outlive its own upgrade in the API and the Nodes page. Dropping the columns is a ~90-site edit across the specs, the e2e suite, the MCP dto and the UI; it is recorded as a follow-up rather than folded in here. A persistent tunnel 401 still does not trigger re-registration, and now for a reason rather than a deferral. Register CLEARS the node's replica rows, so re-registering on a 401 would delete a live worker's rows on every retry, and under the name collision that causes the 401 the two workers would take turns doing it forever: a credential failure causing model reclamation. It also cannot fix the named cause, since a collision is indistinguishable from a restart. The 401 log now names both causes and says nothing can reach this worker, which is true only now that it has no listener. The container healthcheck did not break the way the brief expected, since the listener still exists on loopback and the probe runs inside the container. It did have a real #10987 defect that this change makes the common case: it read LOCALAI_SERVE_ADDR only, while effectiveBasePort reads LOCALAI_ADDR first, so a worker on a non-default base port was probed on 50050 and reported unhealthy while working. It follows the same precedence now. Docs, the compose file and the e2e harness are updated in step: no inbound rule or published port is needed for a worker, the two advertise variables are gone, the remaining address variables are read for their port only, the firewall-the-file-transfer-port warning is narrowed to the LOCALAI_HTTP_ADDR opt-out, and the upgrade-order note no longer claims the worker still listens. The Nodes page showed node.address, which is now always blank, so it shows the node id instead. Eight mutations, all red on a named spec, including reverting the loopback bind, re-adding the address to the registration body, restoring both node-address fall-backs, dropping the force-clear, storing the endpoint's address again, and un-fixing the healthcheck. One of them caught a defect in a spec I had just written: it asserted 200 where the endpoint returns 201, which went unnoticed because core/http/endpoints/localai is not on the task's verify list. It is run here. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…was refused Review round 1 on the change that stopped workers listening. One blocking item and seven notes. LOCALAI_WORKER_TUNNEL=false was the blocking one, and the ruling was to make it fatal rather than to correct the comment that still promised it fell back to the advertised address. There is no fallback left: a worker on this branch advertises nothing and binds only loopback, so turning the tunnel off leaves it reachable by nothing while it registers, heartbeats and reports healthy, and the scheduler keeps placing models on it. That is the worst available failure shape, so a new Config.validateStartup refuses it before prefetch, registration and NATS, while the worker is still invisible to the cluster. It absorbs the pre-existing empty-registration-token check, which had the same shape and no spec. The flag is kept rather than deleted so an operator who set it is told the promise is gone instead of having the setting ignored, and the guard around StartTunnel is removed, because a branch nothing can take reads as a supported no-tunnel mode that does not exist. The justification for erroring on an install that names no address was wrong, and the review is right that this is the dangerous form of overclaiming, because the conclusion holds and the mechanism does not. It said the resulting empty target would be refused as an invalid stream and that the refusal would read as the worker answering about its backend. Nothing in this repo branches on cluster.ErrNoRoute, and nodes.unroutable treats any recorded dial error as unroutable, so that refusal reaches every reap guard as ProbeUnknown and deletes nothing. The site now stands on what holds, that an install naming no port produced nothing routable and the failure belongs to the install rather than to a later probe, and records the retracted claim so nobody re-derives it. This retracts the same paragraph in the body of 1cf847f. The reviewer deleted the whole tryWarmPath unnamed-replica guard and the suite stayed green, including the reservation release. It is specced now, and the asymmetry the review asked about is decided at the site: the row stays, unlike the sibling !alive branch which removes it. That branch has observed a backend dead; this one has observed only that the row is unreadable, which says nothing about whether a process is running, and the row is the last record that one might be, since the acknowledged stop path refuses a stop whose ExpectedAddress does not match and an empty one cannot be cleaned up through it either. The cross-version wire claim rested on two struct tags nobody asserted: renaming only the json keys survived mutation while the gorm column rename went red through raw SQL. Both keys are pinned now, marshal and unmarshal, per struct. A worker-first upgrade showed the operator a status code and not the reason. The registration client discarded the body, so "address is required for backend workers" was read off the socket and thrown away, and the ladder then spent four minutes on a verdict the frontend reached instantly. Refusals now quote the body and carry ErrRegistrationRejected, and both the ladder and the credential manager's Acquire stop on the first one. Acquire matters more than the ladder: it is the default path and its bound is 100 attempts, not 10. 408 and 429 are deliberately not refusals, since both are the frontend asking for the same request again. Also: the stale "not blocked by firewalls" troubleshooting line, which now names the real cause and the knobs that move the port range; and the inert address fields on the MCP Node DTO, which the Assistant was still being handed. The review named http_address there and I removed address too, because it is inert by the same argument and leaving one of a pair is arbitrary. Five mutations, all red. Deleting the warm-path guard reddens four specs and falsifying only its reservation release reddens one, so the two halves are pinned separately. Renaming only the json keys reddens both wire suites. Discarding the refusal body reddens two. Dropping the rejection classification does not fail the suite, it hangs it, which is the operator-visible symptom, so it is recorded red under a ginkgo timeout. The verify list is now derived from the diff rather than from the brief, which is what let the previous round ship a spec asserting 200 where the endpoint returns 201: nine ginkgo suites, the e2e vet, route auth coverage, the leaf check, build, the healthcheck shell suite and lint. The two jsx files have no harness in this worktree and are recorded as the one unverified surface. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…erence Everything this phase built was proven by unit and integration specs. This is the first run of it against the real binaries: a frontend replica per process, a worker that binds nothing routable, real inference over the result. Four scenarios, each with the question "what would make this pass if the tunnel were doing nothing" answered rather than left open. A worker with no advertised address is reached through its tunnel. The roster is asserted to report it advertising nothing, so there is no address a frontend could have dialled instead, and node_connections is asserted to name the replica that serves the request. A request landing on the replica that does NOT own the worker is relayed to the one that does. With N replicas behind round robin that is (N-1)/N of production traffic, so it gets the FIRST request for its model: the backend install, the file staging on the http tag, and the gRPC load and predict all cross the relay. Which replica owns the tunnel is read from the ownership table through the production Owner query and mapped to a frontend index through the address the harness pins per replica; the non-owner is derived from that reading and asserted to be a non-owner immediately before the request, rather than assumed from the harness default. Sending the same request to the owner reddens it. Killing the owning replica re-homes the worker onto the survivor. The worker dials a balancer rather than a replica, because LOCALAI_REGISTER_TO is resolved once at boot and is the tunnel endpoint as well as the registration one: aimed at a single replica, a worker has nowhere to reconnect to when that replica dies, and the re-home cannot happen at all. Removing the kill reddens it. And the negative control for the whole suite, which is why the other three mean anything. Frontend and worker share a host here, so every backend port the frontend names in a stream target is one it could have dialled directly; if it did, the first three would pass with the tunnel inert. LOCALAI_WORKER_TUNNEL is no longer usable for this, because it is a fatal startup error and a worker that never started says nothing about a worker reachable some other way. The balancer answers the tunnel connect path itself instead, leaving a worker that registers, heartbeats, reports healthy and holds no tunnel. It is asserted to have dialled and been refused, asserted to be held by nobody, and then asserted unreachable with the refusal naming the missing route. Then the block is lifted, nothing else changes, and the same request succeeds: that is what attributes the refusal to the tunnel rather than to any of the ordinary reasons an e2e inference fails. The fifth spec measures the head-of-line blocking this phase deferred three times. 128 MiB crosses the session while a warm model is probed back to back, direct and relayed. Median latency is unchanged, the worst probe is about 3x the baseline median and about a seventeenth of the transfer window, and the transfer runs at 415-490 MB/s direct and 222-268 MB/s relayed. A session that head-of-line blocked would park a probe for the length of the window. Leave the yamux windows untuned; and note this is loopback, so it says the multiplexing does not serialise and says nothing about a link with a bandwidth-delay product. The load spec is measured against a control that the first version did not have. It passed with the bulk artifact cut to 4 KiB, because the window it read probes against was mostly cold-load overhead: it would have reported a clean bill on a session carrying no large message. The same cold load now runs twice, once empty and once bulk, and the difference between the windows is asserted to be real before any latency is read from it. Two defects on the base commit came out of this. cluster_peerlink_test.go has been red since the relay landed, deterministically, in isolation and in the suite. It asserted that an accepted peer stream is refused at once, on the premise that phase 1 installs no relay. The relay correctly waits fifteen seconds for a frame naming the worker, and the spec's budget was five. It now writes a relay request for a node no replica holds and asserts the refusal is ErrNotOwner and specifically not ErrNoConnection, which is a stronger spec than the one it replaces and the only thing in the e2e suite that exercises the relay's refusal path. The harness handed a worker's own HTTP port to a backend process. It took two ports from freeport and used one as the gRPC base and the other for the file transfer server; freeport returns adjacent ports often, and the backend allocator hands out base, base+1, base+2, so the second backend started on a worker was regularly given the HTTP server's port and died with EADDRINUSE. No spec had started two backends on one worker before, so it had never fired; the load spec starts five and it failed about one run in three. Each worker now reserves a contiguous bind-probed block laid out the way production lays it out, below the kernel's ephemeral range, with LOCALAI_GRPC_MAX_PORT bounding the allocator to it. The underlying production defect is not fixed here and is recorded in the report: allocatePort never checks that a port is free, and its default range overlaps the ephemeral range on every Linux box. Constraint 6, whether distributed mode should now refuse to start without an advertised address, is DEFERRED, and the comment and the docs that described the cost were understating it. A replica with no advertised address writes no instances row, and Owner joins a connection against a live instance, so a worker whose tunnel lands there is unroutable from every OTHER replica while being registered and healthy. Refusing to start would still be wrong, because the deployments it would break are single-host ones with no peers to be unreachable by, and telling those apart at startup is a design with its own specs. Both places now say what actually happens. Suite wall clock 592s for 15 specs, up from 502s for 10 of which 2 were red. The CI budget of 20 minutes does not move. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Review round 1 on the end-to-end proof. Zero blocking items, eleven non-blocking, and three of them turned out to be production defects rather than notes on the report. The one that matters is a misclassification the phase is built to prevent. A dial carries the caller's deadline down to the socket, so when the budget runs out the socket's timer fires and the error travels back up through the WebSocket handshake and the multiplexer. The context's cancellation is a separate timer whose func the scheduler has to run before ctx.Err() stops returning nil, and nothing orders the two. Under contention the socket's error is back in PeerPool.Open first, ctx.Err() reads nil, and a peer that is listening and healthy is reported as ErrPeerUnreachable to a caller that simply ran out of time. An unreachable peer is a fact a caller may act on and an expired deadline is not, and core/services/nodes routes around a replica it is told is unreachable. callerRanOut answers that question in one place: ctx.Err() when it is set, and otherwise the wall clock against the caller's own deadline. That is sound because it is the same instant the socket compared itself against, so if the socket's timer fired this comparison is past it too. The ambiguous instant resolves towards the caller, which is the direction that never blames a peer. The spec that caught it, peerlink_test.go's "blames the caller's deadline", was red in three of seven -race runs and had been since Task 5, which is often enough to read as noise and is why single-run verification never saw it. Rather than leave the proof to a coin flip, a second spec makes the window deterministic: Open is handed a context whose deadline has passed and whose cancellation has not been delivered, against an address nothing is listening on, so the dial fails for real. It reddens without the fix. The peer link's yamux windows were applied to one end only. A receive window is advertised by the side that RECEIVES, so configuring the dialler alone tunes exactly one direction, and the direction left on the 256 KiB default is the one that carries a relayed model artifact INTO the replica that owns the worker's tunnel. That is the largest thing the link ever moves and it is the direction the load measurement exercises: the review read it as flowing toward the dialler and it does not. PeerLinkConfig is now exported and used on both ends. Measured, same box, 128 MiB staged through the relay against the same transfer without one: the relayed path cost 1.6x to 2.0x the direct path's transfer window before, and 1.06x to 1.25x after. The SSRF reachability spec could be fooled into reporting an SSRF that did not happen. It bound the victim on 127.0.0.2 at an ephemeral port and required 127.0.0.1 at the same port to refuse, so any other spec in the run holding that number made the dial succeed; red one run in seven, green five of five in isolation. It now picks from below the kernel's ephemeral range, the same fix the harness got for the adjacent-port collision. The rest are the specs and the report saying what they mean. Scenario 1's advertisement assertion could not tell "the worker advertises nothing" from "the JSON key moved", which matters because removing the advertisement is the change it covers. It was green against a renamed key. The roster now keeps the raw key set beside the decoded fields and the spec requires both keys present before reading them as empty. Scenario 4's refusal-body check was a four-way disjunction admitting bare "tunnel", "not connected" and "unroutable". Those alternatives were inert and each would be satisfied by refusals that say nothing about routing, in the one assertion the whole negative control rests on. It is "no route" alone. The head-of-line gate bounded the worst probe by the whole transfer window, which admits about eightfold degradation and loosens as the box slows. It is now half the window, plus a scale-free ratio against the worst probe under the SAME cold load with nothing to transfer, which is the control that isolates the transfer from the load. Not tighter than that, and the reason is measured rather than cautious: under a concurrent -race suite the worst relayed probe reached a fifth of its window, so a quarter-window gate would have had 1.2x of margin, and a spec that fails one run in three is worse than no spec. The report entry printed p90 and p99 off samples of twenty, where both land on the same element and p99 often lands on the max, so one number appeared three times under three names. A quantile is now printed only when the sample can separate it. Two claims in the report were wrong and are withdrawn rather than softened. Scenario 2's race is closed by the trailing re-read of the owner, not by the pre-assertion the report credited: a move to the non-owner mid-request would serve directly and still return 200, and only the trailing read reddens on it. And "the median request is unchanged" holds on this box and not on the reviewer's, where the relayed median rises up to 82% and p99 up to 3.5x. What survives on both is structural: the worst probe is a small fraction of the window in which bytes are moving, so the session interleaves rather than serialising. Sharing a session with a bulk transfer costs latency; it does not cost service. The disk footprint note undercounted, and the reviewer lost a run to a full disk on this box, so it is worth having right: two bulk models seeded into two frontends and staged to the worker is about 768 MiB, not 512 MiB. Left alone deliberately: the worker's backend port allocator still hands out ports without checking they are free, and its default range still overlaps the kernel's ephemeral range. It is confirmed, it is out of scope here, and it is being tracked as a named follow-up rather than fixed under an e2e task. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…ckend A worker that refuses a stream has answered, and cluster.Dial keeps the three tunnelproto sentinels out of the ErrNoRoute umbrella precisely so a consumer can act on that. No consumer did. Since workers stopped listening, a backend process that crashed on a healthy worker is no longer a dead listener's codes.Unavailable: the worker refuses the stream with ErrStreamTargetUnavailable, gRPC flattens it into Unavailable anyway, and nodes.unroutable reported the whole thing as "this frontend has no route". Every reap path then answered ProbeUnknown and left the row, so the replica slot never freed and at the default MaxReplicasPerModel=1 the only cleanup left was LRU eviction of models that were working. isWorkerAnswer is exported as cluster.IsWorkerAnswer, so the errors the dialer keeps out of the umbrella are by construction the errors the consumers treat as the worker answering. nodes.unroutable and pkg/model's transportFailure both use it; ConnectionEvictingClient, the site reached during inference, goes through transportFailure rather than asking the transport directly. A reply code this frontend does not recognise is still not an answer, so a newer worker's vocabulary costs a retry and not a replica. The reap guards keep the allow-list rather than requiring ErrNoRoute: an unrecognised dial error must mean "no route", never "the backend is gone". Also in this final pass over the branch: - Docs: recommend upgrading FRONTENDS first, with the symptom of each order. Workers-first fails now that a 4xx registration is a verdict rather than an outage, so an old frontend's "address is required for backend workers" makes each restarted worker exit and drains the fleet a node per restart. - Docs: LOCALAI_WORKER_TUNNEL=false is a fatal startup error, not a degraded mode, in both places that described it; and a frontend rollback needs every worker restarted, because re-registration force-clears the address columns. - A replica with no advertised address now says so every five minutes and names the workers only it can reach, instead of one startup warning for a cost paid for the life of the process. - callerRanOut's rule now holds at all three siblings, so an expired caller deadline stops reading as a broken tunnel; probeHealth's withdrawn reason for using the raw client is corrected; the dead DoOrCached is deleted and its coverage kept on DoOrCachedResult; sweepLeakedInFlight enumerates the outcomes that reach it. - The peer route's self-declared id is recorded as a phase-3 deferral, in the handler, in the isolation claim it narrows, and in the operator docs. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Making a worker's refusal reaping evidence created a defect one layer along, at the producer. The worker refused a ReadStreamRequest failure with ErrStreamRequestInvalid and its own comment said "Includes the deadline above expiring", which was harmless while every refusal reached the frontend as "no route" and became a reap the moment one of them did not. So a request frame that had merely not ARRIVED yet was reported as a non-transient verdict about a backend. It is reachable on the relay path, which carries most production traffic: the worker's header timer starts when the OWNING replica opens the stream, while the frame is written by the DIALLING replica only after the relay's acceptance travels back to it, so a whole peer-link round trip runs inside that window, on a link this design deliberately loads with multi-gigabyte artifacts beside token streams. For a long-deadline caller the endpoint is ConnectionEvictingClient, which stops the model across the fleet. It also falsified the "neither clears on its own" argument that licensed the reap. There is now a fourth refusal, ErrStreamNotServed, for what a worker could not serve for a reason of its OWN. It is deliberately outside IsWorkerAnswer, so it reaches a consumer under the no-route umbrella and reaps nothing, which is the same treatment an unrecognised code already gets. Four producers move onto it: a request frame that timed out (a malformed one stays a verdict, because that is a frontend bug no retry fixes), both SetReadDeadline failures, which are facts about the stream and not about a target nothing has dialled yet, and WriteStreamRefusal's default for a reason nobody classified. classifyServiceFailure keeps ErrStreamTargetUnavailable as its default on purpose: inverting it would make errno enumeration the single point of failure for the reap, and a miss there is a row nothing can ever delete. What it gains is a deny-list of two causes that are provably this worker's own clock or its own context. Also: - The read-site caller-deadline guard in the handshake was unpinned: the existing seam spends the budget before the handshake starts, so only the write could ever fail. A spec whose deadline falls between the request and the reply pins it, and each guard now reddens on its own. - The documented worker-first failure line omitted the JSON error envelope the old frontend returns, so an operator grepping it found nothing. - The peer-link disclosure names the aimable per-session receive window in all four places, and LastDialErrorOf records why a third consumer must go through IsWorkerAnswer rather than roll its own list. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The worker re-classified a failure a local service had already classified. classifyServiceFailure preserved exactly one of the four refusal codes, which was faithful to its own comment for as long as there was one worth keeping; once ErrStreamNotServed existed, a service returning the code whose whole job is to say "I learned nothing" had it promoted to ErrStreamTargetUnavailable, which every reap guard acts on. ErrStreamTagUnknown was promoted too, and cost nothing only because both sides of that one reap. No in-tree service produces either, which is the same "unreachable, therefore safe" argument that let the request-frame merge survive a whole phase, and LocalService is exported. The cause was a fifth site enumerating the vocabulary by hand, so the fix is one table. streamRefusals pairs each sentinel with its wire code and with whether a frontend may act on it as evidence about a backend, and the writer, the reader, IsWorkerAnswer and the new IsStreamRefusal all read it. A fifth code is now taught to every one of them at once. The codes are also pinned against literals written out in a spec, the way this branch already pinned the NATS vocabulary. The round-trip table cannot see a rename, because a rename moves the writer and the reader together; an unrecognised code is deliberately not the worker's answer, so renaming "unavailable" would turn every crashed backend on a tunnelled worker into a row nothing can ever reap, silently and with the suite green. Three comments the previous fix falsified, corrected: - tunnelHeaderTimeout still said the window bounds only framing the frontend writes immediately after opening the stream. That is true on the direct path and false on the relay path, and it was the argument for treating an expiry as the frontend's fault. - classifyServiceFailure's deny-list is three causes, not two: on a dial error net.Error.Timeout also covers ETIMEDOUT and EAGAIN. Both are kept deliberately, because reaping a wedged or resource-starved backend is the eviction this phase exists to prevent, and ECONNREFUSED still reaps. isReadTimeout is renamed reportsTimeout, which is what it asks. - The operator table named three refusals and said a refusal is acted on. It now lists four, with when each is sent and whether the row is reaped. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Releasing a worker tunnel deleted its node_connections row, so "this worker's link dropped a moment ago" and "this worker has never connected here" were one observation: no row. Nothing above could tell a worker re-homing between replicas from a worker that is gone, and any grace period built on top would have had nothing to measure from. The row now survives a departure. Release clears owner_instance_id and stamps disconnected_at on the database clock; the membership sweep and Deregister do the same for every connection a dead or departing replica held; Claim clears the stamp in the same upsert that writes the owner, so a reconnect is never observed half-applied. PurgeDepartedBefore deletes a departure once it is older than DepartedRetention, and the membership tick owns that schedule. Owner and OwnerRow report a departed row as ErrNoConnection, through the one predicate connectionIsHeld, the way instanceIsLive is the one predicate for replica liveness. This change records the departure and does not interpret it: how long ago it happened is nobody's answer yet. The sweep only clears rows that are still held. An empty owner is in no instance's id, so without that filter every heartbeat would restamp every departed row and no departure could ever age out. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The membership tick's call to PurgeDepartedBefore was the only production wiring this change introduced, and removing it left the suite green. A retention nothing applies is a departure that never ages out, which is the state the sweep's held-ness filter exists to make reachable at all. A spec now ages a released row past DepartedRetention on the database clock, starts a real Membership, and waits for the row to go. Release and Deregister leaned on "no owner id is ever empty" to avoid touching an already-departed row, which is the accident Owner refuses to lean on. A departed row keeps its epoch and carries an empty owner, so a release or a deregistration naming an empty id matched it and stamped a fresh departure over the old one, making a worker that left long ago look like one that has only just gone. Both now filter on connectionIsHeld. The comment on DisconnectedAt claimed a held row never carries a departure. A binary from before this column existed claims without clearing the stamp, so a rolling upgrade produces exactly that row. The comment now says what holds, and says to ask held-ness first and read the stamp second. The sweep's vocabulary follows the code: it records departures where the comments still said it deleted rows, and its log line separates the instance rows it deleted from the connection rows it left behind. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
A worker whose tunnel is gone is not, by that fact, a worker that has left. Absence is what makes the scheduler stop placing work, reap the worker's rows and evict its models, and one of those paths runs during inference, so the deployment needs to tell a worker re-homing between frontend replicas from one that is really gone before anything acts. Registry.Presence answers that in one joined statement, with four values and not a boolean: unknown when there is no row at all (this package cannot tell a worker that has never dialled from one whose departure aged out, and must not guess), connected while a live replica holds the tunnel, reconnecting while the departure is inside the grace, and gone once it is older. Only the last is a verdict a caller may act on. Held-ness is asked FIRST and the departure only refines it, in the SQL and again in the switch that reads it. Every writer here clears disconnected_at in the statement that writes the owner, but that is a property of these writers rather than of the table: a replica running a binary from before the column existed re-claims without clearing the stamp, so during a rolling upgrade a held row carries an old departure, and a read that consults the stamp first reports a connected worker as gone for the whole upgrade. Both windows are computed by the database, for the reason every other window in this package is: they are compared across replicas, and replicas disagreeing about whether a worker is gone is the flapping this branch exists to remove. No behavioural spec can see the difference, since the test container shares the host clock, so the statement shape is pinned instead. The grace is an operator's knob, defaulting to twice the worker tunnel's maximum reconnect backoff. That made the fixed departure retention wrong: an operator raising the grace past it gets a purge that deletes departures before the grace elapses, so a worker that is gone reads as unknown forever and nothing ever reaps it. The retention is now derived from the grace, with the old constant as its floor. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The retention a worker's departure is kept for is now derived from the reconnect grace, so a purge can never outrun the window Presence measures against. Nothing pinned that. The sweep could be reverted to pass the constant, or the setter emptied out, and the suite stayed green either way: the specs covered the arithmetic helper, and the fix is the wiring. The loop now has a spec of its own. It departs two workers either side of the difference between the floor and the derived retention, and the row that must go is what witnesses the sweep running at all, so the row that must stay cannot survive by nothing happening. The default grace goes from 60s to 90s. Two of the worker's ceiling backoffs is 60s, but the failed dial between them costs its handshake timeout too, which puts the worst case at 70s, and the backoff resets only after a session long enough that a replica accepting a dial and then dying denies it. So the ceiling is reachable exactly during the rolling restart this window exists for, and 60s sat on the edge of it. Too short reports a live worker as gone and costs a model reload; too long reaps a dead one later. The cheaper mistake is the long one. A held row whose owner is dead and whose stamp is stale is the state a rolling upgrade actually produces, and it was the one state no spec built. It has an answer now, and the two ways to get this wrong land either side of it: reading the stamp first says gone, reading held-ness without the liveness join says connected. Two comments claimed more than the code did. There IS a grace at which a live worker is reported as gone, which is the point of it being a duration; and the switch that reads held-ness first is only a partial second gate, since with the SQL gate gone and a dead owner it answers gone rather than reconnecting. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Ten NATS subscriptions on the worker become ten HTTP routes under
/v1/control/, served on the loopback HTTP server the worker already runs
and reached only through the tunnel's existing `http` stream tag.
The carrier is the tag that already exists rather than a new one. A new
tag would have had to invent correlation, per-request deadlines,
unbounded payloads and a progress stream, and each of those is a place
this branch has already put a defect. It would also have added a fifth
entry to the worker's stream-refusal vocabulary, which decides what a
frontend reaps on and took eight fixes to settle. Riding `http` means a
control RPC to a worker another replica holds takes the same relay the
inference path takes, which is the path that has been measured.
The request and reply DTOs are untouched, so a body on a control route
is byte-for-byte what the corresponding subject carried. No subject was
deleted: agent workers still subscribe to nodes.<id>.backend.stop.
Install and upgrade stream. They answer application/x-ndjson: zero or
more {"progress":...} lines carrying the same event the per-op NATS
subject carried, then exactly one {"reply":...} line, always last. That
deletes the 8000-byte notification cap structurally instead of
reproducing it on a new carrier: a progress line is written into the
response the caller is already reading, so there is nothing to size and
no subscribe-before-request window. The debouncer is shared with the
NATS publisher rather than forked, so the ~4/s tick bound is one fact.
A verb's own failure is a 200 with Error set, never a 5xx. The frontend
maps a transport failure onto "no route to that worker", which nothing
may act on, and the worker's answer onto evidence a reap guard may act
on; answering 500 for a failed install would put the worker's verdict
in the bucket reserved for a broken link. Only a request that could not
be read or routed is non-2xx.
Control RPCs carry the caller's budget. r.Context() replaces four
context.Background() calls at the gallery-install sites, and the one
pre-existing fixed timeout on model.unload is now derived from the
caller's context so a shorter budget is honoured. No timeout is invented.
The inner `go func()` in the install and upgrade handlers is deleted
rather than nested: it existed because one subscription served every
install, and over HTTP each request already has its own goroutine.
Per-backend serialization stays lockBackend, which is what actually
prevented two requests racing the gallery directory.
Bounds against a boundary the worker now serves: every body is capped at
8 MiB before any decode; the 404 echoes at most 128 bytes of the request
path, cut on a rune boundary so a half rune cannot travel downstream as
a replacement character; non-POST is refused before the body is read so
a probe cannot fire a command; the streaming responses set nosniff.
The routes mount through nodes.AuthenticatedRoutes, which hands the
registrar a private mux and puts the whole prefix behind the same
constant-time bearer check as the file routes. The worker's HTTP server
now takes the supervisor as a required parameter, so there is no way to
start it without the control plane mounted.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…publisher Review fix round 1. Seven non-blocking findings; the blocking one is a merge gate for Task 4 rather than anything in this diff, and the report's concern about it is corrected: until Task 4 lands, PingNode probes two subjects no serve-backend worker subscribes to any more, so every healthy worker reads as absent and is marked unhealthy on the scheduling path. The rune-boundary cut in truncate was true behaviour with nothing holding it: a byte-wise mutation survived all 201 specs. isRuneStart is replaced by utf8.RuneStart, the same predicate the cluster package uses for this rule, and two specs pin it, one with a rune straddling the bound and one with a rune ending exactly on it so the fix cannot be "always walk back". unloadModel answered Success:true whatever Free did. That is the worker saying "done" about work it did not do, and the frontend's only caller is EvictLRU, so a false yes told the scheduler VRAM had been released and let it place the next model on a node still holding the old one. It now reports the failure, following stopModelExact, which is the honest pattern already in this package. Still a 200: the worker answered, only its verdict is negative. An address with nothing loaded still answers success, which is a true answer rather than a claim about work done. NewDebouncedInstallProgressPublisher had no production caller after the last commit, only its own spec. Deleted rather than wired: wiring it would publish every event on two carriers, which is what the carrier decision exists to avoid. Its specs now run against the sink, plus one that pins the identity stamped on each event, since the subject used to carry the op and node id and now nothing but the body does. The install progress wiring was exercised by no spec, because with no gallery nothing ever invokes the download callback. The guard moves into startProgress, shared by install and upgrade, which also emits one resolving event before any gallery work. That is worth having on its own: a cold install spends minutes on a manifest and a progress stream with nothing on it is indistinguishable from a broken one. It also makes the wiring observable end to end, and four specs now drive the real installBackend and upgradeBackend over HTTP with no override. model/stop and backend/stop keep taking Background rather than the caller's context, and the sites now say why. model/stop is the acknowledged stop path: it reserves the process, frees it, kills it, waits for exit and releases the port, and abandoning that because the caller hung up would leave a process marked stopping, a port not returned to the allocator and a row nothing reconciles. In stopBackendExact the Free is a courtesy before a kill that happens anyway. model/unload differs because Free IS the operation there. A route set with no prefix or no registrar is now a startup error rather than a silent no-op: a server that comes up healthy while every route the caller registered answers 404 is, through a tunnel, indistinguishable from a version skew. And the AllPaths spec no longer claims to catch a constant that was never added to the set, which it cannot; it asserts the whole set instead, which catches a verb dropped from it. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The ten backend and model lifecycle verbs stop being NATS requests and become HTTP calls on the worker's own control routes, reached through that worker's tunnel on the `http` stream tag that already carries file staging. Nine subject builders and the per-op install-progress subject are deleted with their entries in the worker's NATS permissions; the request and reply DTOs are untouched, so a body on the wire is byte for byte what the subject carried. This closes the merge gate Task 3 left open, which was worse than lost commands. Once the worker stopped subscribing, PingNode was still asking nodes.<id>.backend.list and nodes.<id>.models.running, so EVERY healthy worker answered no-responders, nodeAnswersOnBus read it as absence and pickReachableNode demoted it on the scheduling path. PingNode is a control RPC now, and no control RPC can produce ErrNoResponders, which is the only error that exclusion acts on. Two specs drive pickReachableNode against a real adapter and a worker answering over its control plane, which is the only arrangement that can see the difference: the router's own double never touches a transport and stayed green for the whole window the defect was live. How a control RPC FAILS is the whole of this change, so it is decided in ONE function reading ONE table. A worker's answer passes through unwrapped, so cluster.IsWorkerAnswer still sees it and a reap guard may act on it; everything else is wrapped in ErrWorkerUnroutable so nothing can. There is no third branch, because a third branch is how the eight collapses on this branch happened: each was a site that decided for itself which errors were evidence. A 404 under the prefix is its own sentinel, because it is the worker stating a deployment fact about ITSELF rather than a verdict about a backend, and only the legacy upgrade fallback may act on it. The caller's budget is checked FIRST. A timeout is not a verdict: a refusal arriving in the instant a deadline expires would otherwise be reported as the worker's non-transient answer, which reaps a row, and nothing orders the two timers. A 5xx and an undecodable body are transport failures, not answers. An empty ModelsRunningReply means "this worker is running nothing", which the reconciler acts on, so it must never be manufactured from a body that would not parse. A stream that ends before its reply line is the same rule one layer up: a tunnel dying mid-install is not the worker saying the install failed. backend.stop is split by node type rather than moved. Agent workers hold no tunnel, so they have no control plane to serve, and they still subscribe to nodes.<id>.backend.stop to drop cached MCP sessions; that subject and its agent permission both survive. It is the honest intermediate state until agent workers hold tunnels too. A failed control RPC no longer demotes a node anywhere. ErrNoResponders meant "not on the bus"; a control failure means "this frontend could not route to it", which is equally what a healthy worker re-homing its tunnel between replicas produces. Absence is a fact read from the database, and the scheduler starts reading it in a later task. The rolling-update fallback re-fires a DESTRUCTIVE force-reinstall, so it runs only on the worker's own 404. Its negative direction was pinned at the admin call site and unpinned at the reconciler's, where widening the condition to any error left all 676 specs green: a background drain nobody is watching would then force-reinstall every queued backend the moment a replica lost its tunnels. Three specs cover it, arranged so the force install IS reachable in the negative case and a fallback that fired would show as a call and a drained row. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…stated Review fix round 1. Two blocking findings and seven non-blocking; both blocking ones are M12's shape again, and this time on the invariant itself. No production behaviour changes here: everything below was already correct and merely unpinned, so re-inserting the defect left all 679 specs green. The only non-comment edits are one struct-field comment and one log message. "A failed control RPC no longer demotes a node" is stated three times in this package and was pinned once, at ListBackends. Putting MarkUnhealthy back at either op-drain site passed. What that buys in production is the fleet-wide eviction this phase exists to prevent: MarkUnhealthy removes a node from ListDuePendingBackendOps AND from scheduling, so a frontend replica that has just lost its tunnels demotes every node it holds an op for, for a reason that is about the frontend. The reconciler's is the worse of the two, being a background loop nobody is watching. Both now have a spec, each with the recorded op failure as its negative control so "still healthy" cannot pass by nothing having happened. The sweep the review asked for found four more rules stated at more call sites than they were pinned at, and two the review had not: The still-installing surfacing at the manager layer has two call sites and was pinned at InstallBackend. Dropping it from UpgradeBackend reported a spent budget as GREEN SUCCESS: the admin sees the upgrade finished while the worker is still re-pulling gigabytes. The agent-node skip has two call sites and was pinned at ListBackends. Without it the fan-out enqueues a row for every agent node, and an agent worker serves no control plane, so that row can never drain: it retries until the dead-letter cap. The still-installing conversion has three call sites and was pinned at two; the legacy force-install fallback was the gap. Its budget was unpinned too, so the new spec asserts both, on the upgrade budget rather than the install one, since the fallback re-fires an install as part of an upgrade. The carrier split has two call sites and was pinned at one. Hardcoding NodeTypeBackend in UnloadRemoteModelContext passed, and an agent node holding a node_models row would then have its stop sent over a tunnel it does not hold, fail, and leave the row behind. The new spec unloads a model held by one node of each kind and asserts each stop went to that node's own carrier and to no other. router_nats_liveness_test.go asserted demote-on-absence, which production can no longer produce, and its header described the pre-cutover world. The exclusion is unreachable by construction rather than by argument: cluster, the package supplying every control-path dial error, does not link nats.go at all. The file now says that, and gains the assertion that IS load-bearing, a table naming each sentinel a control RPC can answer with and requiring that none of them excludes. Widening the exclusion to ErrWorkerUnroutable reddens four of its entries plus the real-adapter scheduling spec. unroutable keeps no budget-first guard and the reason is now written at it: unlike controlFailure it reads one already-recorded error rather than racing a live deadline, and an expiry is not in streamRefusals, so it falls to the umbrella without one. The two implement the same split at two layers and each now names the other. Fourteen comments still described the bus. Among them the reconciler saying a drain would "churn NATS every tick", a spec comment naming a subject builder this branch deleted, and the agent-skip comment explaining the skip by a subscription that no longer exists. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The four nodes.<id>.files.* subjects were the last commands a serve-backend worker took off the bus. They are now HTTP routes under workerctl.Prefix, on the same loopback server and behind the same bearer check as the ten lifecycle verbs, so the frontend reaches them through the worker's tunnel. files.listdir is the verb this matters most for. Its reply had to fit a payload the bus would carry, which put a wide model directory close to the limit; a response body has no such ceiling, so nothing truncates the listing at either end. A short listing reads to the frontend as files the worker does not have. S3NATSFileStager becomes S3FileStager and calls ControlClient, which means every failure now lands in the bucket phase 3 exists to keep straight: a route this frontend could not use is unroutable and nothing may act on it, while the worker's own answer, including "that file is not there", is evidence a caller may act on. Each RPC's deadline is DERIVED FROM the caller's context rather than started fresh, at every one of the five call sites, so a caller that gave up stops the RPC too. A worker started without an object store mounts no file verb at all and answers 404, which is the same answer a build too old to know them gives. The subjects and the backend worker's files.> publish grant go with them; a backend worker now publishes nowhere but its own inbox. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…verbs The rule "a body this worker could not parse is a non-2xx, never the worker's answer" is written at three exits in control_files.go and only ensure was pinned. Turning stage's or listdir's decode exit into a 200-with-error left worker and nodes entirely green, and what that converts is a frontend's malformed request into the worker's own verdict about a file, which passes cluster.IsWorkerAnswer and reaches a reap guard. The production code was already right; nothing held it there. The e2e NATS JWT spec was asserting the opposite of the code and passing. It published nodes.<id>.files.in and called it an allowed subject after that grant was deleted, and it could not tell: a permission violation does not close the connection, so FlushTimeout and IsConnected both stay happy. It now reads LastError, the way its sibling always has, and asserts the denial plus the one publish right a backend worker has left. Also pinned, each mutation-verified alone: the CreateTemp branch (an existing staging-tmp at 0500 reaches it without a seam), the walk's context check (a caller that gave up must fail the listing, never be answered with a short one), and the cache and data directory layout. The data directory was derived twice, once in worker.go and once for the listdir verb; worker.go now reads the same helper, so a move cannot leave a verb listing files the file server does not serve. The per-verb RPC ceiling moves from an argument at five call sites into fileRPCBudget, so no site can name the wrong one, and the two values are asserted. The body-cap table now holds both directions locally and with two different claims: a body exactly at the cap proves the bound is a ceiling and not an off-by-one, and an absolute megabyte proves the cap stays above real gallery traffic. Only the second notices a cap shrunk to 64 KiB. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…bus timeout The scheduler decided whether a worker had gone away from nats.ErrNoResponders: one frontend's observation that nobody answered IT within a request budget. Two replicas asking in the same moment could disagree and demote each other's workers, and a worker re-homing its tunnel between replicas looked identical to one that had died. SmartRouter now reads cluster.Presence instead. Only PresenceGone -- no live replica holds the tunnel AND the departure has outlived the reconnect grace -- excludes a node from placement, and it is a fact every replica reads identically from the database. PresenceReconnecting, PresenceUnknown and a failed presence query are all non-verdicts and place work as normal: excluding on a database hiccup would cost the fleet its capacity for a reason that has nothing to do with any worker. nodeAnswersOnBus is deleted. It excluded on a sentinel no control RPC can produce, so it decided nothing while PingNode cost a relayed round trip per scheduling decision to feed it. PingNode goes with it, from the adapter and from NodeCommandSender. isRequestTimeout drops nats.ErrTimeout: every verb this adapter sends now travels over the worker's tunnel. The predicate is named nodeMayTakeWork rather than nodeHasRoute. "Route" is ErrWorkerUnroutable in this package, the condition nobody may act on; PresenceGone is the one a scheduler may. Spelling them the same way is the collapse this work exists to prevent. Also folds in ReapStale's return rename: it counts connection rows CLEARED, never rows deleted, and reading it as a delete count would make a worker that is re-dialling right now look forgotten. The spec pinning that a message merely quoting "nats: timeout" is not a timeout was scripting a SUCCESSFUL reply carrying the phrase, which comes back with a nil error and never reaches the classifier. Restoring the string match left it green. It now scripts a 5xx whose body carries the phrase, and asserts that the phrase reaches the classifier as a precondition. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…all sites Removing "Presence: clusterRegistry" from the options literal in initDistributed left all seven suites and tests/e2e/distributed green. The predicate was right and its input was silently nil, which returns the deployment to absence being decided by nothing, with no log line and no failing request. That is the fourth finding of this exact shape in this phase. The two assignments move out of a twenty-field literal into distributedSchedulerOptions, a named function a unit spec can reach. Deleting either is now red. The health monitor takes its presence reader and grace as a required positional pair instead, so deleting those does not compile at all. requireAbsenceWiring then refuses to start a distributed frontend whose scheduler or health monitor has no source of absence, because refusing to boot is the only symptom either failure has. With a fresh heartbeat and a permanently gone tunnel there was no reaper at all. A heartbeat says the worker's supervisor is alive; it says nothing about whether anything here can reach that worker's backends, because those are reached over the tunnel. A proxy that stops upgrading WebSockets, a rotated registration credential or a reconnect loop longer than the grace left a node listed healthy forever while every request for a model already loaded on it failed "no route to that worker", and every reaper keyed on the heartbeat. The health monitor now reads presence from the same place and against the same window as the scheduler and demotes such a node. That also ends the 15s re-promotion: the demotion arm returns before the recovery arm, so the scheduler's demotion is no longer undone on the next tick, and recovery needs the tunnel back rather than just the heartbeat. The demotion is status-only. MarkOffline would DELETE the node's rows, and deleting rows on a presence read would give any future defect in that read the widest blast radius in the system for nothing the demotion does not already deliver. LRU eviction is the third path that commits work to a node, and it read only the stored status. A node full enough to be an eviction target is exactly the node the VRAM and idle selectors never offer, so pickReachableNode structurally cannot cover it. It now runs its chosen node through the same nodeMayTakeWork predicate, demotes it and evicts again rather than handing back an install that cannot land. Presence is read after the transaction and not inside it: reading it inside would hold a FOR UPDATE lock across a query needing a second pooled connection, which is how concurrent evictions deadlock a pool. Also: a router built with a presence reader and no grace now has its documented default pinned by a spec rather than only claimed by a comment; ageDeparture asserts RowsAffected, since an UPDATE matching nothing succeeds and the inside-the-grace spec returned the same verdict either way; the scheduler comment that still described the bus is corrected; the docs stop conflating heartbeat recovery with tunnel recovery and name the third reader; and an overlong rewrapped line in membership.go is folded. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
A local-ai worker no longer opens a bus connection. connectNATS and its
spec are gone; Run registers once, starts its tunnel, arms /readyz on that
tunnel, and heartbeats. The worker's bus credential flags (--nats-jwt,
--nats-user-seed, --nats-require-auth, the three TLS flags) and
Config.NatsAuthRequired go with it. --nats-url stays, accepted and
ignored, so an existing worker command line still parses.
/readyz was the thing most likely to wedge a tunnel-only worker: it
required a live NATS link, so a worker with no bus would have reported
itself unready forever. nodes.NATSReadiness becomes nodes.TunnelReadiness
over a local interface{ Connected() bool }, and worker.Tunnel gains
Connected(), backed by a mutex-guarded session field the loop publishes
and clears. A closed-but-not-yet-cleared session reads as disconnected:
the loop waits for every in-flight stream before it clears the field, and
the probe must answer not-ready through that wait.
The heartbeat gate is DELETED rather than re-pointed at the tunnel. The
heartbeat is the worker's own answer that its process is alive; whether
the frontend can reach it is a separate fact the frontend already holds
and ages against LOCALAI_WORKER_RECONNECT_GRACE. Withholding the
heartbeat would report an unreachable worker as an absent one on the one
path with no grace, where the health monitor marks it offline and its
pending backend ops are deleted behind it. heartbeatLoop is given no view
of the tunnel, so a gate cannot be added back without changing its
signature.
Removing the NATS credential manager from this path also removes a defect
it carried: its refresh loop re-registered on a timer to renew a JWT, and
Register CLEARS a node's NodeModel rows. Any backend worker running on
frontend-minted credentials had its replica rows deleted roughly every
18 hours.
Of core/cli/workerregistry, everything survives. The manager is still
used in full by core/cli/agent_worker.go, which still needs NATS: Acquire,
Provider, RefreshLoop, HasCredentials and TunnelToken are all untouched.
The backend worker simply calls RegisterFullWithRetry directly now.
WorkerPermissions is documented as serving agent nodes, and its non-agent
branch narrowed to _INBOX.> on both sides. It is NOT deleted: NATS reads
an empty allow list as no restriction, so returning nil would upgrade
every JWT the frontend still mints for a backend node from its own inbox
to the whole account.
Agent workers keep the bus everywhere: their CLI flags, their
subscriptions, the agent branch of WorkerPermissions, and the compose
service with its LOCALAI_NATS_URL and depends_on: nats.
Also corrected two flags the Nodes page advertised that do not exist
(--distributed-nats, --distributed-db), and a log line plus several
comments that still named a bus the code no longer touches.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Five cluster specs that run the binaries an operator runs, plus the repair of eighteen specs phase 2 left red. The eighteen were router_tracking and full_flow, failing since 1cf847f on "reported backend installed but named no address for the process". Two contracts had changed under them: an install reply that names no worker-local address is refused rather than substituted, and a frontend with no worker dialer reaches no backend at all. Nobody noticed for a phase because phase 2 verified with --label-filter='Cluster', which excludes both suites. ServeBackendLifecycle and tunnelBackendClients state both facts once for every spec. The transport double is the part that matters. It translates a refused connect into cluster.ErrStreamTargetUnavailable, which is what a real worker answers when its backend process has died and what IsWorkerAnswer lets a reap guard act on. A bare ECONNREFUSED reaches those guards as "no route" and reaps nothing, so a double returning the raw syscall error could never fail the way production fails; putting it back reddens the stale-record spec and nothing else. The new specs cover: a backend worker with no bus URL in its /proc environ registering, being scheduled onto and serving inference; a backend install and a backend listing driven through the replica that does NOT own the worker, with the owner read through the production Owner query and re-read after; that install's progress proven to arrive before its terminal reply, made deterministic by a gallery server that holds the worker's fetch open so a reply cannot exist yet; a worker whose tunnel is genuinely gone, waited for rather than assumed, losing nothing inside the reconnect grace and re-homing after; a heartbeating worker with a permanently dead tunnel losing its healthy status while an agent worker in the same cluster keeps it; and the suite's negative control, where a control RPC to a tunnel-less worker fails naming the missing route, reaps nothing, and succeeds the moment the tunnel returns. Every scenario was attacked. The churn one was WRONG on the first attempt and only the attack found it: its hold window sat entirely inside cluster.InstanceLiveness, so a killed replica still read as a live owner throughout, presence was "connected", and the spec passed with the reconnect grace set to a nanosecond. It now blocks the tunnel before the kill and waits for the ownership row to actually empty. Attacks that redden the rest: posting at the owner, writing the install reply before the work, collapsing PresenceReconnecting into PresenceGone, removing the non-backend node-type guard, and not blocking the tunnel. Agent workers turn out to be protected twice over; no single mutation reaches them. Harness: Options.AgentWorkers and Options.ReconnectGrace, WorkerEnviron (read from /proc, because Cmd.Env is the harness agreeing with itself), NatsURL, FrontendBackendsDir, AgentWorkerName, PostJSON, and a node String() so a failing roster assertion is readable instead of several hundred bytes rendered as numbers. Budget: 20 specs at 787 to 808 seconds over three runs, up from phase 2's 591 to 612. --timeout goes to 30m so a loaded runner reports a cause rather than a spec name. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
| // observed". | ||
| func reserveWorkerPorts() (int, error) { | ||
| for attempt := 0; attempt < workerPortAttempts; attempt++ { | ||
| base := workerPortFloor + rand.IntN(workerPortCeiling-workerPortFloor) |
| // is up by finding it in the roster. | ||
| func (c *Cluster) startAgentWorker(i int) (*Process, error) { | ||
| name := agentWorkerName(i) | ||
| cmd := exec.Command(c.opts.Binary, "agent-worker") |
| } | ||
| for file, content := range c.opts.Models { | ||
| path := filepath.Join(dir, "models", file) | ||
| if err := os.WriteFile(path, []byte(content), 0o644); err != nil { |
| d = scaled | ||
| } | ||
| } | ||
| return d/2 + time.Duration(rand.Int64N(int64(d/2)+1)) |
| return fmt.Errorf("tunnel frame is %d bytes, over the %d-byte limit", len(payload), maxTunnelFrame) | ||
| } | ||
| buf := make([]byte, 2+len(payload)) | ||
| binary.BigEndian.PutUint16(buf[:2], uint16(len(payload))) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Workers no longer need inbound ports
Distributed mode used to require every worker to expose an inbound address that the frontend dialled directly. This inverts that: a worker dials the frontend load balancer over HTTP and holds one multiplexed yamux tunnel. That tunnel lands on exactly one frontend replica, and every other replica reaches the worker by relaying through the owner. Workers now bind loopback only and advertise nothing.
A worker needs one outbound HTTPS connection to the same load balancer a browser would use. No routable address, no open ports back into the worker's network.
This also brings the distributed end-to-end suite into CI, which is what made the rest of the change reviewable at all.
NATS is untouched here. This is phase 2 of the programme to remove it; phases 3 to 6 move the control plane, backend installs, fan-out and the claim queue.
How it works
GET /api/cluster/connectand holds a yamux session. gRPC, HTTP and websocket traffic are multiplexed over it.Operator impact
LOCALAI_ADVERTISE_ADDRandLOCALAI_ADVERTISE_HTTP_ADDRare no longer used.LOCALAI_WORKER_TUNNEL=falseis now a fatal startup error rather than a degraded mode, because there is no direct-dial path left and a worker started that way would register healthy and be permanently unreachable.The invariant this rests on
Four failures must never be reported as each other: a routing fact, an absent connection, an unreachable peer, and an infrastructure error. Absence makes the scheduler act, reaping rows and evicting models, and one of those paths runs during inference. Removing the direct-dial fallback means a collapse between them stops being degraded and becomes unrecoverable.
Review found and fixed eight instances of that collapse: at the
cluster/nodespackage boundary, at five separate reaping sites, in three client decorators, in the model loader and the inference-path evicting client, in the peer link (where an expired caller deadline surfaced as "peer unreachable" under contention, reproducible in three of seven race runs), and one introduced by the fix for the fifth. WhatDialexcludes from the "no route" umbrella and what consumers exempt from "unroutable" are now one exported predicate over one table, so the two cannot drift.CI and testing
The distributed suite previously started a PostgreSQL and a NATS container per spec, roughly 48 minutes of pure container startup for 213 specs, which is why it was excluded from CI. It now uses one container per test process with a database per spec and runs in about 75 seconds. The same change applied to the shared test helper cut the cluster suite from 97s to 37s, jobs from 34s to 3s and agents from 14s to 2s.
New end-to-end coverage proves inference over the tunnel, over the relay to a non-owning replica, and re-homing after the owning replica is killed, with a negative control that makes the other three meaningful: the tunnel is blocked at the balancer and the no-inbound-ports worker must be unreachable, then the block is lifted and the identical request succeeds.
Head-of-line blocking was measured rather than assumed: 128 MiB across the session while a warm model is probed. The worst probe is between a seventh and a nineteenth of the transfer window, so the session interleaves. This is loopback, so it says nothing about a link with a real bandwidth-delay product, and the yamux windows are deliberately left at their defaults pending that data.
Bugs found in existing code, reported not fixed
LOCALAI_AUTH_HMAC_SECRET.HealthCheckIntervalandStaleNodeThresholdhave config fields but no flag or env binding.allocatePortallocates from bookkeeping only and never checks a port is free, and its default base sits inside Linux's ephemeral range.-racefailures unrelated to this branch:core/services/galleryop/cancellable_phase_test.go:192, andpkg/modelprocess_exit_test.goviaxlog.SetLogger.Known follow-ups, deliberately not here
/api/cluster/peertakes a self-declared replica id, so anything holding the shared registration token can relay to every worker a replica owns, evict its inbound link, and aim a large per-session buffer at it. This is not a regression in kind: before this change the same token reached every worker's advertised gRPC and file-transfer ports directly. Closing it properly needs a credential minted where a replica joins the instances table, which is a migration and a design.BackendNode.AddressandHTTPAddressremain as inert columns; removing them touches roughly 90 sites.LastDialErroron the backend interfaces would let embedding promote it and delete the unwrapper machinery entirely.|| node.idfallback.🤖 Generated with Claude Code
https://claude.ai/code/session_01Y2TjpXdY7SszRrM5PhSp1e